Skip to content

Conversation

@crisap94
Copy link
Member

@crisap94 crisap94 commented Jan 7, 2026

Summary

After 4 failed attempts with OIDC trusted publishing (#20, #21, #22, #23), we're switching to the reliable traditional npm token authentication method.

What Happened with OIDC

Despite correct configuration, OIDC consistently failed:

Configuration was verified correct:

  • ✅ Trusted Publisher configured: ubidots/react-html-canvas/deploy.yml/npm-production
  • ✅ Repository is public
  • ✅ id-token: write permission set
  • ✅ Provenance signing worked every time

New Approach: Traditional npm Token

This PR implements the proven traditional authentication method.

Changes

  • ✅ Add registry-url: 'https://registry.npmjs.org' back to setup-node
  • ✅ Add NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} to publish step
  • ✅ Bump version to 0.2.12
  • ✅ Keep --provenance flag for supply chain security

Required Setup (Action Needed!)

Before merging, you need to create the NPM_TOKEN secret:

  1. Create npm automation token:

  2. Add GitHub secret:

Benefits

  • Reliable: Battle-tested authentication method
  • Provenance: Still supports supply chain security with --provenance flag
  • Works: No configuration mysteries or OIDC issues

Trade-offs

  • ⚠️ Manual token management: Need to create and rotate tokens
  • ⚠️ Security: Token could be compromised if leaked (use GitHub secrets properly)

Testing Plan

Once NPM_TOKEN secret is configured:

  1. Merge this PR
  2. Workflow triggers automatically
  3. Should successfully publish v0.2.12 to npm
  4. GitHub release created automatically

Status: ⏸️ Waiting for NPM_TOKEN secret to be configured before merging

Summary by CodeRabbit

  • Chores
    • Version bumped to 0.2.12.
    • Deployment workflow updated: registry URL configured for npm and publish step adjusted to use an auth token; provenance flag removed from publish command.

✏️ Tip: You can customize this high-level summary in your review settings.

After 4 failed attempts with OIDC trusted publishing (#20, #21, #22, #23),
we're switching to the more reliable traditional npm token authentication.

Changes:
- Add registry-url back to setup-node
- Add NODE_AUTH_TOKEN env using NPM_TOKEN secret
- Bump version to 0.2.12
- Keep --provenance flag for supply chain security

Required setup:
1. Create npm automation token at https://www.npmjs.com/settings/[username]/tokens/create
2. Add as GitHub secret: NPM_TOKEN

Benefits:
✅ Battle-tested and reliable
✅ Still supports provenance attestation
✅ No OIDC configuration issues

Trade-offs:
⚠️ Requires manual token management
⚠️ Token needs rotation
@coderabbitai
Copy link

coderabbitai bot commented Jan 7, 2026

📝 Walkthrough

Walkthrough

Updated CI deploy workflow to specify the npm registry URL and provide an auth token for publishing; package version bumped from 0.2.11 to 0.2.12.

Changes

Cohort / File(s) Summary
CI/CD Configuration
\.github/workflows/deploy\.yml
Added registry-url: 'https://registry.npmjs.org' to Node.js setup; removed --provenance from npm publish and added env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} for authenticated publishing.
Package Metadata
package\.json
Bumped version from 0.2.11 to 0.2.12.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Poem

🐇 I hopped to the registry, a token in paw,
publish prepared with a tiny new law.
Version nudged upward, a neat little climb,
CI hums softly, in rhythm and time. 🚀

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly indicates the main change: switching from OIDC to traditional npm token authentication, which aligns with the core objective of the PR.
Description check ✅ Passed The description is comprehensive and covers the motivation, changes made, setup instructions, and trade-offs. It follows the general spirit of the template with clear sections and actionable information.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2d71254 and c293d40.

📒 Files selected for processing (1)
  • .github/workflows/deploy.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/deploy.yml

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Jan 7, 2026

🔍 PR Quality Check Summary

Check Status
Code Quality & Tests ✅ success
Security Audit ✅ success

📋 Checks Performed:

  • ✅ TypeScript compilation
  • ✅ ESLint code quality
  • ✅ Prettier code formatting
  • ✅ Unit tests with coverage (80%+ required)
  • ✅ Build verification
  • ✅ Security audit

🎉 All checks passed! This PR is ready for review.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @.github/workflows/deploy.yml:
- Line 79: The publish step currently uses the `--provenance` flag in the
command string `npm publish --access public --tag ${{ steps.version.outputs.tag
}} --provenance`; remove the `--provenance` flag so the publish runs with
`NODE_AUTH_TOKEN` token-based auth (i.e., change the run command to omit
`--provenance`) to avoid OIDC/provenance conflicts that cause the publish to
fail.
🧹 Nitpick comments (1)
.github/workflows/deploy.yml (1)

11-11: Consider removing the id-token: write permission.

The id-token: write permission was required for OIDC-based publishing but is no longer needed with traditional npm token authentication. Keeping it doesn't cause harm but removes unnecessary permissions following the principle of least privilege.

🧹 Proposed cleanup
 permissions:
-  id-token: write
   contents: write
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 017bf84 and 2d71254.

📒 Files selected for processing (2)
  • .github/workflows/deploy.yml
  • package.json
🔇 Additional comments (3)
package.json (1)

3-3: LGTM! Version bump is correct.

The patch version increment to 0.2.12 follows semantic versioning appropriately for this workflow configuration change.

.github/workflows/deploy.yml (2)

63-63: LGTM! Registry URL correctly configured.

The npm registry URL is properly specified for traditional token authentication.


80-81: Configuration is correct—verify NPM_TOKEN secret exists before deployment.

The NODE_AUTH_TOKEN environment variable is properly configured for npm authentication. The setup-node action (line 59–63) with registry-url: 'https://registry.npmjs.org' creates the necessary .npmrc that respects this token, which matches npm and GitHub Actions documentation.

Before this workflow is deployed, ensure the NPM_TOKEN secret has been created at the repository level with a valid npm automation token.

@github-actions
Copy link

github-actions bot commented Jan 7, 2026

🔍 PR Quality Check Summary

Check Status
Code Quality & Tests ✅ success
Security Audit ✅ success

📋 Checks Performed:

  • ✅ TypeScript compilation
  • ✅ ESLint code quality
  • ✅ Prettier code formatting
  • ✅ Unit tests with coverage (80%+ required)
  • ✅ Build verification
  • ✅ Security audit

🎉 All checks passed! This PR is ready for review.

@crisap94 crisap94 merged commit 8ab3e6c into main Jan 7, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants